Check PSReadLine version during PowerShell integration install#30
Check PSReadLine version during PowerShell integration install#30caomengxuan666 wants to merge 3 commits into
Conversation
|
Given the minimum requirement of PowerShell 7.4 (= PSReadLine 2.3.4), do we still need this shim? |
|
Yeah, fair question. I think the 7.4 floor is for PSNativeCommandPreserveBytePipe rather than PSReadLine itself. This wasn't meant as support for pwsh < 7.4. The issue seems to be that a supported pwsh can still load an older user-installed PSReadLine instead of the bundled one. The reporter said they're on pwsh 7.6.2, but PSReadLine 2.1.0 is what gets loaded. So if the intended support policy is really "pwsh 7.4+ with its bundled PSReadLine", I'm fine closing this as an unsupported environment. Otherwise this guard seems like a small defensive fix for a pretty bad failure mode. Also, is the 7.4+ requirement mainly for PSNativeCommandPreserveBytePipe? If yes, I get why that's needed for full correctness. I was only wondering whether older PowerShell could still have a best-effort mode, since many Windows sandboxes still only have Windows PowerShell 5.1. |
|
I think it may be best to check for the PSReadLine version in the installer and abort it if it doesn't meet the requirement. People may not be aware that they're on an older version (because they have installed it as a local module years ago) and continue to miss out on important updates for years to come, without ever realizing it. |
That makes sense. I can change this into an installer-side PSReadLine version check and fail the PowerShell integration with a clearer message. One caveat is AllUsers installs: the installer can only check the module resolution for the account running it, while another user may still have an older user-scoped PSReadLine. So an installer check catches the common case, but a small runtime guard would still prevent the profile hook from breaking another user's prompt. |
Oh, that's a great point. Let's keep the guard in then! I'll give it another thought soon. (I'm currently swamped with work, so I'll take a short break from this project though.) |
|
Sounds good, thanks. I'll keep the guard as-is for now. |
| function Assert-PSReadLineRequirement { | ||
| try { | ||
| $module = Import-Module PSReadLine -PassThru -ErrorAction Stop | Select-Object -First 1 | ||
| } | ||
| catch { | ||
| throw "PowerShell integration requires PSReadLine $MinPSReadLineVersion or newer, but PSReadLine could not be loaded: $($_.Exception.Message)" | ||
| } | ||
|
|
||
| if (!$module -or !$module.Version) { | ||
| throw "PowerShell integration requires PSReadLine $MinPSReadLineVersion or newer, but the resolved PSReadLine module did not report a version" | ||
| } | ||
|
|
||
| if ($module.Version -lt $MinPSReadLineVersion) { | ||
| $path = if ($module.Path) { " from '$($module.Path)'" } else { '' } | ||
| throw "PowerShell integration requires PSReadLine $MinPSReadLineVersion or newer, but PowerShell resolved PSReadLine $($module.Version)$path. Update or remove the older PSReadLine module and re-run the installer." | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't think we'll need this right? The installer now checks the PSReadLine version, so this is redundant?
There was a problem hiding this comment.
Agreed. I removed the runtime overload fallback and kept the installer-side PSReadLine version check as the single guard.
I also updated the PR title/body so it no longer claims to fall back at runtime.
There was a problem hiding this comment.
But it's this, the Assert-PSReadLineRequirement check, that I don't think serves a purpose. Am I missing something?
There was a problem hiding this comment.
Ah, got it.
I was thinking about the #20 case, where the user had a supported PowerShell but an older PSReadLine was still resolved first.
So this check was meant to fail earlier with a clearer message before installing the hook, not to support old PowerShell.
But maybe that is too defensive. If you prefer treating it as an environment issue, I’m fine closing this PR.
caomengxuan666
changed the title
2 participants
Fixes #20.
The injected
PSConsoleHostReadLineoverride uses the 3-argumentPSConsoleReadLine.ReadLine(..., lastRunStatus)overload. Older PSReadLine versions do not have that overload; their 3-argument overload takes aCancellationToken, so the profile hook can throw on every prompt read.This checks the resolved PSReadLine version during PowerShell profile installation and fails the integration with a clear message when it is older than 2.3.4, matching the PSReadLine version bundled with PowerShell 7.4.
Validated locally by:
src/pwsh-install.ps1with PowerShell's parsersrc/pwsh-install-template.ps1with PowerShell's parserpwsh -NoProfile -NonInteractiveand checking the resolved version/path